West Midlands | ITP-Sept-25 | Mustaf Asani | Sprint 3 | Coursework/sprint 3 implement and rewrite#795
Conversation
|
Your PR's title isn't in the expected format. Please check the expected title format, and update yours to match. Reason: Sprint part (Sprint-3) doesn't match expected format (example: 'Sprint 2', without quotes) If this PR is not coursework, please add the NotCoursework label (and message on Slack in #cyf-curriculum or it will probably not be noticed). |
cjyuan
left a comment
There was a problem hiding this comment.
The files in the Sprint-2 folder are not related to Sprint-3 exercise. Can you revert the changes made in the Sprint-2 folder?
Sprint-3/1-implement-and-rewrite-tests/implement/2-is-proper-fraction.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/2-is-proper-fraction.test.js
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js
Outdated
Show resolved
Hide resolved
…orrected test description for invalid cards, used more informative descriptions
… for cards with more than 1 digit, removed check for negative value since im using maths.abs
cjyuan
left a comment
There was a problem hiding this comment.
There are still three modified files in the "Sprint-2" folder. Can you revert the changes made to them?
Sprint-3/1-implement-and-rewrite-tests/implement/2-is-proper-fraction.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js
Outdated
Show resolved
Hide resolved
| expect(isProperFraction(7, 4)).toEqual(false); | ||
| expect(isProperFraction(35, 8)).toEqual(false); |
There was a problem hiding this comment.
Could also test some negative improper fractions in this test.
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js
Show resolved
Hide resolved
…nnecessary if statements
…be converted to numbers.
cjyuan
left a comment
There was a problem hiding this comment.
-
The tests are now very solid. Well done.
-
This PR branch still contains unrelated changes in "Sprint-2" folder. Can you revert the changes made in the "Sprint-2" folder to make the PR branch clean?
Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js
Outdated
Show resolved
Hide resolved
cjyuan
left a comment
There was a problem hiding this comment.
Changes look good. Well done.
| if (strictNumber(rank)) { | ||
| if (pattern.test(Number(rank))) { | ||
| return Number(rank); | ||
| } | ||
| } |
There was a problem hiding this comment.
Is the outer if statement necessary?
|
|
||
| if (faceCardPattern.test(rank)) { | ||
| return 10; | ||
| } else if (/^A$/.test(rank)) { |
There was a problem hiding this comment.
Could also just write rank == 'A'.
Learners, PR Template
Self checklist
Changelist
created functions and practiced test driven development by making tests for the functions.